feat: Show friendly help when run with no arguments#52
feat: Show friendly help when run with no arguments#52HuiNeng6 wants to merge 1 commit intosystemblueteam:mainfrom
Conversation
- Make file_a and file_b optional arguments - Show full help when no args provided (exit 0) - Show clear message when only one file provided - Add file existence validation with helpful paths Fixes: systemblueteam#49
📝 WalkthroughWalkthroughThe CLI argument handling is refactored to make Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sounddiff/cli.py (1)
127-135: 🧹 Nitpick | 🔵 TrivialConsider consistent Rich formatting for error messages.
The new validation code uses Rich's
console.printwith colored output, but these exception handlers still use plainclick.echo. This creates inconsistent error styling for users.Additionally, PR objective
#4(wrong file type) requested clearer messages before lower-level errors. TheValueErrorhandler at lines 130-132 catches format errors fromload_audio(), but passes through the raw exception message without Rich formatting.♻️ Optional refactor for consistent Rich formatting
except FileNotFoundError as e: - click.echo(f"Error: {e}", err=True) + console.print(f"[red]Error:[/red] {e}") sys.exit(1) except ValueError as e: - click.echo(f"Error: {e}", err=True) + console.print(f"[red]Error:[/red] {e}") sys.exit(1) except RuntimeError as e: - click.echo(f"Error: {e}", err=True) + console.print(f"[red]Error:[/red] {e}") sys.exit(1)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: efdec335-1cfb-4a3b-a977-66eda1505e34
📒 Files selected for processing (1)
src/sounddiff/cli.py
| Compares FILE_A (reference) against FILE_B (comparison) and reports | ||
| differences in loudness, spectral content, timing, and potential issues. | ||
| """ | ||
| console = Console() |
There was a problem hiding this comment.
The error console doesn't respect --no-color.
The Console() instance created here will always output colors, ignoring the --no-color flag. The progress console at line 120 correctly passes no_color=no_color.
🐛 Proposed fix
- console = Console()
+ console = Console(no_color=no_color)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console = Console() | |
| console = Console(no_color=no_color) |
|
Console() on line 68 doesn't pass no_color=no_color, so --no-color gets ignored for these error messages. Line 120 already does it right. We fixed this same thing in #40. |
|
path.exists() returns True for directories too. If someone passes a directory by mistake they'll get a confusing soundfile error instead of the friendly message. path.is_file() is the fix for both checks. |
systemblueteam
left a comment
systemblueteam
left a comment
There was a problem hiding this comment.
Two small fixes needed, see comments above.
2 participants
Improve CLI user experience for first-time users.
Changes:
Fixes: #49